Skip to content

[Components][Form] Fixed version specific examples of some form events. #3886

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

ahsio
Copy link
Contributor

@ahsio ahsio commented May 28, 2014

Q A
Doc fix? yes
New docs? no
Applies to 2.3
Fixed tickets
  • Examples that point to DataCollectorListener (added since 2.4) should be removed (or adapted when it's possible).
  • This fix only applies to 2.3.

@weaverryan
Copy link
Member

You're right! Since these are just examples to help you understand the purpose of what a listener might be doing on each even, what if we just added a very small note about these being a 2.4 feature? I don't think mentioning the listener in the 2.3 docs is a problem (as long as we avoid causing any confusion by pointing out that you won't see this listener unless you're looking at the 2.4 codebase).

What do you think @ahsio?

weaverryan added a commit that referenced this pull request Jun 7, 2014
…r form extension (xabbuh)

This PR was merged into the 2.4 branch.

Discussion
----------

[Components][Form] add versionadded for the data collector form extension

| Q             | A
| ------------- | ---
| Doc fix?      | yes
| New docs?     | no
| Applies to    | 2.4+
| Fixed tickets |

As @ahsio pointed out in #3886, the data collector extension was added to the Form component in Symfony 2.4.

Commits
-------

beceed4 add versionadded for the data collector form extension
@ahsio
Copy link
Contributor Author

ahsio commented Jun 16, 2014

@weaverryan I do agree with you. @xabbuh 's commit should then be applied to 2.3+

Also, 2.3 links to DataCollectorListener class should point to the 2.4 version of the API.

@weaverryan
Copy link
Member

@ahsio So, after merging #3887, I actually think things are almost perfect. As I mentioned, I don't mind having the notes there, even on 2.3. But you're right that the API doc links will 404 on 2.3, which I didn't think of before. So, I think what we need to do is remove the API links on 2.3 (or all versions, I don't think they add much here) or convert them to code links (e.g. a link to that file on GitHub).

What do you think?

@wouterj
Copy link
Member

wouterj commented Aug 15, 2014

+1 for leaving the note in here
+1 for removing the API link in 2.3
-1 for using code links

@ahsio can you please update? (if you don't have time, please say so, somebody else will do it then)

@wouterj wouterj mentioned this pull request Sep 16, 2014
@wouterj
Copy link
Member

wouterj commented Sep 16, 2014

Replaced by #4237

@wouterj wouterj closed this Sep 16, 2014
weaverryan added a commit that referenced this pull request Oct 2, 2014
This PR was merged into the 2.3 branch.

Discussion
----------

Finished #3886

Replaces #3886

Commits
-------

01057ae Reverted removal, removed API links instead
0670f25 [2.3] Examples that points to the DataCollectorListener should be removed.
@ahsio ahsio deleted the fix-added-example-to-wrong-version branch December 10, 2014 15:08
@ahsio
Copy link
Contributor Author

ahsio commented Dec 10, 2014

@wouterj @weaverryan Sorry for not being able to adresse your questions on time! Thanks for fixing the issue!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants